Skip to content

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Sep 17, 2025

This changes the token type functions such that they no longer create a
new array of token types every time, but instead use normal equality
checks and includes on a pre-computed set.

acorn token types can be mutated, but we already take a snapshot of
the types at time of creating our AcornTypeScript. Due to this, we can
just store the Object.values one time up front and call includes on
the same array later.

All of this means we no longer cause garbage collection (for creating a
single-use array) and we save CPU cycles on calling includes against
sets if we already matched.

This changes the token type functions such that they no longer create a
new array of token types every time, but instead use normal equality
checks and `includes` on a pre-computed set.

`acorn` token types can be mutated, but we already take a snapshot of
the types at time of creating our `AcornTypeScript`. Due to this, we can
just store the `Object.values` one time up front and call `includes` on
the same array later.

All of this means we no longer cause garbage collection (for creating a
single-use array) and we save CPU cycles on calling `includes` against
sets if we already matched.
Copy link

changeset-bot bot commented Sep 17, 2025

🦋 Changeset detected

Latest commit: 2c51818

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/acorn-typescript Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@manuel3108
Copy link
Member

Looks like linting is failing on file that was not modified here 🤔

@benmccann
Copy link
Member

Looks like linting is failing on file that was not modified here 🤔

fixed in main and then merged main into this branch to fix it. I'll have to leave the actual review for someone else

Copy link
Member

@baseballyama baseballyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! This makes sense.

There’s no doubt this will improve performance, but we won’t know by how much until we actually measure it. However, in any case, since this is a very simple fix, I think it’s fine to merge it.

@dummdidumm dummdidumm merged commit 1f06249 into sveltejs:main Sep 26, 2025
@github-actions github-actions bot mentioned this pull request Sep 26, 2025
@43081j 43081j deleted the garbage-man branch September 26, 2025 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants